Skip to content

Conversation

@dmanners
Copy link
Contributor

@dmanners dmanners commented Apr 15, 2017

Description

Since Zend Framework 1 is at end of life we should remove the usage of Zend_Json. This review is part of #9236

Fixed Issues (if relevant)

  1. Upgrade ZF components. Zend_Json #9236: Upgrade ZF components. Zend_Json (Part fix)

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@dmanners
Copy link
Contributor Author

I was seeing issues with some test cases under /Magento/Weee/Test/Unit/Model/Attribute/Backend/Weee/TaxTest.php it seems that

$this->scopeOverriddenValue = $scopeOverriddenValue
is causing issues as this is the tax model's parent but the tax model does not deal with the attribute ScopeOverriddenValue

@okorshenko okorshenko self-assigned this Apr 17, 2017
@okorshenko okorshenko added this to the April 2017 milestone Apr 17, 2017
@okorshenko
Copy link
Contributor

@dmanners thank you for reporting about Magento/Weee/Test/Unit/Model/Attribute/Backend/Weee/TaxTest.php. We will fix it

@magento-team magento-team merged commit f327e9e into magento:develop Apr 19, 2017
magento-team pushed a commit that referenced this pull request Apr 19, 2017
 - fixed incorrect constructor in Tax model
magento-team pushed a commit that referenced this pull request Apr 19, 2017
magento-team pushed a commit that referenced this pull request Apr 19, 2017
magento-team pushed a commit that referenced this pull request Apr 19, 2017
@magento-team
Copy link
Contributor

@dmanners thank you for your contribution to Magento 2 project

@dmanners dmanners deleted the remove-zend-json-from-weee branch July 26, 2017 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants